doc/git-workflow.md: Rewritten based on new experience #835

lukego · 2016-03-18T06:08:12Z

I rewrote the git-workflow explanation with the goal of briefly but clearly explaining both how to submit a change to Snabb Switch and also how to be a subsystem maintainer.

This is intended to be a chapter in the manual. I marked [wip] because I haven't written the section about how to send your merged changes further upstream yet.

cc @eugeneia @wingo @kbara what do you think of this as a summary of our recent discussions and experiences?

I rewrote the git-workflow explanation with the goal of briefly but clearly explaining both how to submit a change to Snabb Switch and also how to be a subsystem maintainer. This is intended to be a chapter in the manual.

domenkozar · 2016-03-18T14:44:14Z

src/doc/git-workflow.md

-its development.
+1. Pick your technical area of interest. What kind of changes will you be responsible for reviewing and merging? Try to pick an area that is easy to identify, for example "the packetblaster program", "the Intel I350 device driver", or "the Git Workflow chapter of the manual".
+2. Create a branch with a suitable name on your Github fork, for example `packetblaster`, `i350`, or `git-workflow`. This is where you will merge relevant changes.
+3. Describe this branch in the file `src/doc/branches.md` and open a Github Pull Request. This will kick off two discussions: how to clearly identify the changes that you are responsible for, and to which "next-hop" upstream branch you should send the changes that you have accepted by merging.


Would be nice to know the branch name to open Github Pull Request to start with. master?

We haven't figured that out completely yet. I think in most cases the PRs should be against master, then again some PRs only make sense targeting a specific branch (e.g. PRs related to the IPsec topic branch for instance). I have to update SnabbBot to merge PRs with their target branch instead of a fixed branch (master in our case) for the latter case to work well.

I think to remember that @wingo thinks we should use the target branch to designate the next upstream hop. I don't think this is a good idea because contributors shouldn't be required to know who the next hop is, and if its not the right hop then I think its hard to rectify without closing/opening a new PR.

The nix branch should have a well-defined upstream next-hop where @domenkozar sends his Pull Requests. We maintainers should pick one as part of accepting #836.

I propose that nix feeds upstream to kbara-next. what do you think @kbara and others?

This process is supposed to be explained in point 3 of the text quoted above about updating branches.md. How should we make that clearer?

Note: There are two separate processes both in this document and in actual practice. One is for making an individual contribution, such as a new feature or a bug fix, and one is for sending a subsystem branch upstream, e.g. merging the nix branch with its upstream next-hop.

The document is supposed to clearly explain that there are two processes, one for contributors and one for maintainers, and to spell out the details of each. Obviously it is not doing so perfectly :). How could we improve it?

Maybe there should be a section at beginning explaining the difference between the two so the reader chooses which part to read?

I'm happy to be the upstream for the nix tributary.

eugeneia · 2016-03-18T17:07:09Z

👍 Teeny tiny concern: people might expect to find a drivers branch and wonder what <person-next> branches are about, or even next. Imho this should document the whole picture of the current situation, and be updated accordingly when things establish.

I see minor overlap with #761, or at least this should be linked to from CONTRIBUTING.md.

lukego · 2016-03-20T05:55:20Z

@eugeneia Good point. How about if we start maintaining a map of the current branches and then refer to that where appropriate?

Here is a first attempt and using a little creative license in terms of content:

Source for ditaa:

                                   +--lisper
                   +--max next<----+--documentation<--pdf manual
                   |
       fixes       |
         |         |
master<--+--next<--+--kbara next<--+--nix
                   |               +--mellanox
                   |
                   |
                   +--wingo next<--+--lwaftr
                                   +--multiproc

Contains "XXX rewrite" where old sections have been removed but replacements not written yet.

This file now contains a diagram so it needs to have separate .src.md and .md versions. This can be cleaned up when these changes eventually merge with the on-demand markdown (snabbco#829).

lukego · 2016-03-21T06:10:52Z

Thanks @domenkozar @eugeneia for the feedback. I have the feeling that the first redraft I submitted was not explaining things very clearly. I have pushed another partial rewrite now (half complete, half missing). Are there some parts that are less clearly explained than others now?

I also know that I have a tendency to overuse analogies when trying to elucidate the Git workflow (sorry @wingo...) but I can't look at our branch structure now without thinking of master as a river and the subsystem branches as tributary streams, like in this map from the Wikipedia page about the source of the Rhine river. Is this worth talking about in the text or would it just be distracting?

lukego · 2016-03-21T09:56:28Z

I pushed a new section about how to be an upstream maintainer. I would really like feedback/ack/nack on this from the existing maintainers. cc @eugeneia @wingo @kbara.

Added note to branches.md that branch 'nix' feeds upstream to 'kbara-next' following ack from kbara on snabbco#835.

kbara · 2016-03-21T10:30:56Z

It looks good, though the next-hop part needs filling in. I think that's currently the fuzziest part of the whole workflow to me.

also stopped capitalizing "Pull Request" because it seemed awkward.

lukego · 2016-03-22T09:36:33Z

@kbara Thanks for the feedback! I have written and pushed the next-hop section now. I am sure it is missing a lot of information. Can you tell me how it looks and what is still not clear?

Generally I think that having a well-defined branch tree is the key to making upstreaming work smoothly. This is hard now in the early days because we have so few branches and maintainers. Going forward it seems like if new features are first merged onto an appropriate subsystem branch (e.g. nix, mellanox, pdf-manual, etc) and then make their way up the tree (being sanity-checked and conflict-resolved at each step) then it should be easy to coordinate development.

The hope is that this maintenance structure will be scalable i.e. that we can get more useful work done by adding more maintainers. The barrier of entry to becoming a maintainer should be fairly low because the tree above acts as a safety net and mentoring framework. This seems to be how it works in the Linux kernel world.

One aspect I have not touched on in the documentation is having additional upstream communities around another Github fork e.g. the one on igalia/snabbswitch. This seems like an important mechanism for long term scalability since the rate of notifications from snabbco/snabbswitch is likely to become overwhelming at some point and it will be better to localize specific subsystems to their own repositories. I see a parallel where the Linux Kernel Mailing List was enough in the early days but can't keep up with all the discussions anymore and so these days there are around a hundred more specific mailing lists.

However, I feel that it would be premature to promote wide adoption of that model just yet. I tried creating a separate snabbnfv-goodies repository for the NFV application and that seems to have been counter-productive compared with working directly on this upstream repository because it reduced visibility of the development without commensurate benefits in that particular case.

lukego · 2016-04-29T08:02:12Z

Ready for upstreaming, please!

wingo · 2016-04-29T08:12:28Z

src/doc/git-workflow.md


-This document explains the Git workflows that we use to develop and
-release Snabb Switch.
+How do you engage with the Snabb Switch developer community? The answer depends on what you want to do:


Still uses the name "snabb switch", fwiw, here and below; not a blocker but FYI

kbara · 2016-04-29T13:54:24Z

Is it intentional that this contains both git-workflow.md and git-workflow.src.md?

eugeneia · 2016-04-29T16:10:58Z

I believe this is in need of manual merge conflict resolution.

Resolved conflicts with incoming changes from master branch: *.src.md replaced with simply *.md "Snabb Switch" being renamed to "Snabb"

Ditaa images are not kept in tree anymore...

lukego · 2016-05-02T04:32:35Z

Have merged master and resolved conflicts now. The overall diff seems to reflect only the intended changes against current master.

eugeneia · 2016-05-02T09:38:50Z

For some reason this PR doesn't show up in https://api.github.com/repos/snabbco/snabb/pulls and therefore is invisible for SnabbBot. I will try to close and reopen it just to see what happens.

eugeneia · 2016-05-03T13:47:06Z

Figured it out, see #910

kbara · 2016-06-22T16:33:24Z

Merged, belatedly!

This adds support for YANG's choice statement (RFC6020 Section 7.9) which allows for validation when two blocks can be specified exclusively. The choice block is unique in a few ways and requires the introduction of a new grammar handler. The choice statement is only there for configuration purposes and should not appear itself in the data so it requires some changes allowing for the defined "case" data to essencially replace the choice statement.

doc/git-workflow.md: Rewritten based on new experience

80614e1

I rewrote the git-workflow explanation with the goal of briefly but clearly explaining both how to submit a change to Snabb Switch and also how to be a subsystem maintainer. This is intended to be a chapter in the manual.

lukego mentioned this pull request Mar 18, 2016

src/nix/manual.nix: Nix expression to build the manual #831

Closed

doc/git-workflow.md: Added section on upstreaming subsystems

12d0009

lukego mentioned this pull request Mar 18, 2016

FIX snabbnfv getting-started.md #833

Merged

domenkozar reviewed Mar 18, 2016
View reviewed changes

domenkozar mentioned this pull request Mar 18, 2016

Add nix maintainer branch #836

Merged

lukego added 2 commits March 21, 2016 06:00

doc/git-workflow.md: Partly rewritten draft

af8ae46

Contains "XXX rewrite" where old sections have been removed but replacements not written yet.

doc/git-workflow.md: Created .src.md

eccb614

This file now contains a diagram so it needs to have separate .src.md and .md versions. This can be cleaned up when these changes eventually merge with the on-demand markdown (snabbco#829).

git-workflow.md: Rewrote section on becoming a maintainer

113b607

lukego added a commit to lukego/snabb that referenced this pull request Mar 21, 2016

Merge snabbco#831 remote-tracking branch 'domenkozar/nix' into next

b388a63

Added note to branches.md that branch 'nix' feeds upstream to 'kbara-next' following ack from kbara on snabbco#835.

git-workflow.md: Wrote about upstreaming subsystem branches

4a37b98

also stopped capitalizing "Pull Request" because it seemed awkward.

lukego mentioned this pull request Apr 14, 2016

Describe testing framework for developers #865

Open

lukego changed the title ~~[wip] doc/git-workflow.md: Rewritten based on new experience~~ doc/git-workflow.md: Rewritten based on new experience Apr 29, 2016

wingo reviewed Apr 29, 2016
View reviewed changes

lukego mentioned this pull request Apr 29, 2016

Update CONTRIBUTING.md #904

Merged

kbara self-assigned this Apr 29, 2016

lukego added 2 commits May 2, 2016 04:28

Merge branch 'master' into doc-git-workflow-redux

6964969

Resolved conflicts with incoming changes from master branch: *.src.md replaced with simply *.md "Snabb Switch" being renamed to "Snabb"

doc: Remove stale .images/Branches.png file

6c46b5d

Ditaa images are not kept in tree anymore...

eugeneia closed this May 2, 2016

eugeneia reopened this May 2, 2016

kbara pushed a commit that referenced this pull request Jun 22, 2016

Merge PR #835 (workflow docs) into kbara-next

e7a5471

kbara added the merged label Jun 22, 2016

kbara mentioned this pull request Jun 28, 2016

2016-June improvements #955

Merged

eugeneia merged commit 6c46b5d into snabbco:master Jul 13, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc/git-workflow.md: Rewritten based on new experience #835

doc/git-workflow.md: Rewritten based on new experience #835

lukego commented Mar 18, 2016

domenkozar Mar 18, 2016

eugeneia Mar 18, 2016

lukego Mar 20, 2016

lukego Mar 20, 2016

domenkozar Mar 20, 2016

kbara Mar 21, 2016

eugeneia commented Mar 18, 2016

lukego commented Mar 20, 2016

lukego commented Mar 21, 2016

lukego commented Mar 21, 2016

kbara commented Mar 21, 2016

lukego commented Mar 22, 2016

lukego commented Apr 29, 2016

wingo Apr 29, 2016

kbara commented Apr 29, 2016

eugeneia commented Apr 29, 2016

lukego commented May 2, 2016

eugeneia commented May 2, 2016

eugeneia commented May 3, 2016

kbara commented Jun 22, 2016

doc/git-workflow.md: Rewritten based on new experience #835

doc/git-workflow.md: Rewritten based on new experience #835

Conversation

lukego commented Mar 18, 2016

domenkozar Mar 18, 2016

Choose a reason for hiding this comment

eugeneia Mar 18, 2016

Choose a reason for hiding this comment

lukego Mar 20, 2016

Choose a reason for hiding this comment

lukego Mar 20, 2016

Choose a reason for hiding this comment

domenkozar Mar 20, 2016

Choose a reason for hiding this comment

kbara Mar 21, 2016

Choose a reason for hiding this comment

eugeneia commented Mar 18, 2016

lukego commented Mar 20, 2016

lukego commented Mar 21, 2016

lukego commented Mar 21, 2016

kbara commented Mar 21, 2016

lukego commented Mar 22, 2016

lukego commented Apr 29, 2016

wingo Apr 29, 2016

Choose a reason for hiding this comment

kbara commented Apr 29, 2016

eugeneia commented Apr 29, 2016

lukego commented May 2, 2016

eugeneia commented May 2, 2016

eugeneia commented May 3, 2016

kbara commented Jun 22, 2016